Fix not thread safe xmlSchemaParse calls in ZTS builds by calling xmlSchemaInitTypes during MINIT #20150

SpencerMalone · 2025-10-12T19:15:28Z

See: https://gitlab.gnome.org/GNOME/libxml2/-/issues/930

I was getting a segfault in a ZTS build with:

💣 Program crashed: Bad pointer dereference at 0x0000000000000038

Thread 40 "php-13" crashed:

 0 0x00000001a71a0614 xmlSchemaInitTypes + 1536 in libxml2.2.dylib
 1 0x00000001a7182fb8 xmlSchemaParse + 32 in libxml2.2.dylib
 2 _dom_document_schema_validate + 328 in libphp.dylib at /Users/spencermalone/Projects/php-src-php-8.3.25/ext/dom/document.c:1787:9

  1785│                 (xmlSchemaValidityWarningFunc) php_libxml_error_handler,
  1786│                 parser);
  1787│         sptr = xmlSchemaParse(parser);                                                                                                                                        
      │         ▲
  1788│         xmlSchemaFreeParserCtxt(parser);
  1789│         PHP_LIBXML_RESTORE_GLOBALS(new_parser_ctxt);

 3 execute_ex + 132 in libphp.dylib at /Users/spencermalone/Projects/php-src-php-8.3.25/Zend/zend_vm_execute.h:57068:7

  57066│                 if (UNEXPECTED(!OPLINE)) {
  57067│ #else
  57068│                 if (UNEXPECTED((ret = ((opcode_handler_t)OPLINE->handler)(ZEND_OPCODE_HANDLER_ARGS_PASSTHRU)) != 0)) {                                                       
       │       ▲
  57069│ #endif
  57070│ #endif

 4 ZEND_DO_FCALL_SPEC_OBSERVER_HANDLER + 520 in libphp.dylib at /Users/spencermalone/Projects/php-src-php-8.3.25/Zend/zend_vm_execute.h:2052:4

  2050│                         LOAD_OPLINE();
  2051│                         ZEND_ADD_CALL_FLAG(call, ZEND_CALL_TOP);
  2052│                         zend_execute_ex(call);                                                                                                                                
      │    ▲
  2053│                 }
  2054│         } else {

 5 execute_ex + 132 in libphp.dylib at /Users/spencermalone/Projects/php-src-php-8.3.25/Zend/zend_vm_execute.h:57068:7
 6 ZEND_DO_FCALL_SPEC_OBSERVER_HANDLER + 520 in libphp.dylib at /Users/spencermalone/Projects/php-src-php-8.3.25/Zend/zend_vm_execute.h:2052:4
 7 execute_ex + 132 in libphp.dylib at /Users/spencermalone/Projects/php-src-php-8.3.25/Zend/zend_vm_execute.h:57068:7
 8 ZEND_DO_FCALL_SPEC_OBSERVER_HANDLER + 520 in libphp.dylib at /Users/spencermalone/Projects/php-src-php-8.3.25/Zend/zend_vm_execute.h:2052:4
 9 execute_ex + 132 in libphp.dylib at /Users/spencermalone/Projects/php-src-php-8.3.25/Zend/zend_vm_execute.h:57068:7
10 zend_call_function + 1428 in libphp.dylib at /Users/spencermalone/Projects/php-src-php-8.3.25/Zend/zend_execute_API.c:961:3

   959│                 zend_init_func_execute_data(call, &func->op_array, fci->retval);
   960│                 ZEND_OBSERVER_FCALL_BEGIN(call);
   961│                 zend_execute_ex(call);                                                                                                                                        
      │   ▲
   962│                 EG(jit_trace_num) = orig_jit_trace_num;
   963│         } else {

11 0x00000001013bedc0 zif_frankenphp_handle_request + 900 in frankenphp
12 ZEND_DO_ICALL_SPEC_OBSERVER_HANDLER + 136 in libphp.dylib at /Users/spencermalone/Projects/php-src-php-8.3.25/Zend/zend_vm_execute.h:1400:2

  1398│ 
  1399│         zend_observer_fcall_begin(call);
  1400│         fbc->internal_function.handler(call, ret);                                                                                                                            
      │  ▲
  1401│ 
  1402│ #if ZEND_DEBUG

13 execute_ex + 132 in libphp.dylib at /Users/spencermalone/Projects/php-src-php-8.3.25/Zend/zend_vm_execute.h:57068:7
14 zend_execute.cold.1 + 344 in libphp.dylib at /Users/spencermalone/Projects/php-src-php-8.3.25/Zend/zend_vm_execute.h:61665:2

  61663│         i_init_code_execute_data(execute_data, op_array, return_value);
  61664│         ZEND_OBSERVER_FCALL_BEGIN(execute_data);
  61665│         zend_execute_ex(execute_data);                                                                                                                                       
       │  ▲
  61666│         /* Observer end handlers are called from ZEND_RETURN */
  61667│         zend_vm_stack_free_call_frame(execute_data);

15 zend_execute + 76 in libphp.dylib at /Users/spencermalone/Projects/php-src-php-8.3.25/Zend/zend_vm_execute.h:61648:48

  61646│         }
  61647│ 
  61648│         object_or_called_scope = zend_get_this_object(EG(current_execute_data));                                                                                             
       │                                                ▲
  61649│         if (EXPECTED(!object_or_called_scope)) {
  61650│                 object_or_called_scope = zend_get_called_scope(EG(current_execute_data));

16 zend_execute_scripts + 208 in libphp.dylib at /Users/spencermalone/Projects/php-src-php-8.3.25/Zend/zend.c:1895:4

  1893│                 }
  1894│                 if (op_array) {
  1895│                         zend_execute(op_array, retval);                                                                                                                       
      │    ▲
  1896│                         zend_exception_restore();
  1897│                         if (UNEXPECTED(EG(exception))) {

17 php_execute_script + 540 in libphp.dylib at /Users/spencermalone/Projects/php-src-php-8.3.25/main/main.c:2529:13

  2527│                 }
  2528│ 
  2529│                 retval = (zend_execute_scripts(ZEND_REQUIRE, NULL, 3, prepend_file_p, primary_file, append_file_p) == SUCCESS);                                               
      │             ▲
  2530│         } zend_end_try();
  2531│

Upon digging in, it looks like xmlSchemaParse is only thread safe if you call xmlSchemaInitTypes pre-threading / during init. I left out any call to xmlSchemaCleanupTypes due to 8742276, which would "normally" take care of this kind of thing, and as far as I can tell we haven't been calling cleanup in any way forever, so I'm trying to minimize changes here. If y'all think it's necessary I'm happy to adjust.

Locally, I haven't seen it crop up post-fix, but pre-fix it was occurring ~1/5th of the time when I was replicating by having https://github.com/php/frankenphp run phpunit in multiple threads.

Lemme know if I'm missing anything in making a PR here or if you want anything more rigorous than this.

nielsdos

Thanks!
The proper location for this would be in ext/libxml/libxml.c. The reason is that multiple extensions expose schema functionality, not just DOM.

So the solution is, more specifically, replacing the following code:

php-src/ext/libxml/libxml.c

Line 936 in 8507bcc

xmlInitParser();

such that it calls xmlSchemaInitTypes when libxml is compiled with schemas, and xmlInitParser (like it does now) otherwise. (You don't need the ZTS check for this). Note that xmlSchemaInitTypes will call xmlInitParser.
While at it, you can search the code for more calls to xmlSchemaInitTypes / xmlInitParser and remove them, such that the ext/libxml initialization routine is the only location.

SpencerMalone · 2025-10-12T22:18:01Z

🫡 I can do that, I'll reply when it's ready for a re-review.

SpencerMalone · 2025-10-12T22:36:14Z

Just pushed. Tested locally and when I did xmlInitParser in both parts of the conditional, segfault occurred immediately twice in a row, and then swapped the if to xmlSchemaInitTypes and ran w/o segfault 50 times.

nielsdos · 2025-10-13T19:34:31Z

Thanks! That's what I actually thought of.

Just pushed. Tested locally and when I did xmlInitParser in both parts of the conditional, segfault occurred immediately twice in a row, and then swapped the if to xmlSchemaInitTypes and ran w/o segfault 50 times.

Quite interesting actually. Which version of libxml are you testing on?
I just noticed that xmlInitParser is only called from xmlSchemaInitTypes starting from libxml 2.15. So previous versions would still need the xmlInitParser call.
Are you saying the following does not work? I.e. always calling xmlInitParser unconditionally.

		xmlInitParser();
#ifdef LIBXML_SCHEMAS_ENABLED
		xmlSchemaInitTypes();
#endif

SpencerMalone · 2025-10-13T21:06:43Z

Ah, no, I'm saying for my ease of testing I was doing:

#ifdef LIBXML_SCHEMAS_ENABLED
		xmlInitParser();
#else
		xmlInitParser();
#endif

as my "pre-fix" scenario to make it easy to swap back and forth and reproduce the error + confirm the fix fixes it (in lieu of more repeatable tests).

I can swap to what you're suggesting for older versions though, I can probably get this PR updated and tested with that sometime tomorrow.

nielsdos · 2025-10-13T21:11:15Z

Okay I see!

I can swap to what you're suggesting for older versions though, I can probably get this PR updated and tested with that sometime tomorrow.

Yeah that would be preferable after all. I should've checked the earlier versions too before making my initial suggestion.
After you push I'll test on multiple versions on ZTS to confirm.

…SchemaInitTypes during MINIT. See: https://gitlab.gnome.org/GNOME/libxml2/-/issues/930

SpencerMalone · 2025-10-14T17:32:16Z

Pushed up, tested locally, seems happy with this changeset as well. Lemme know if you want anything else!

nielsdos

This works, thanks.
We may need more initialization calls, e.g. it seems that xmlRelaxNGInitTypes has the same issue. I'll take care of that.
On master we should remove other calls lingering around to xmlInitParser & co. I'll take care of that as well.

Follow-up for phpGH-20150.

Follow-up for GH-20150.

SpencerMalone requested a review from nielsdos as a code owner October 12, 2025 19:15

github-actions bot added the Extension: dom label Oct 12, 2025

SpencerMalone force-pushed the fix-libxml-segfault branch from e0908b7 to 8507bcc Compare October 12, 2025 19:56

nielsdos requested changes Oct 12, 2025

View reviewed changes

SpencerMalone force-pushed the fix-libxml-segfault branch from 8507bcc to 47ebdfc Compare October 12, 2025 22:34

github-actions bot added Extension: libxml and removed Extension: dom labels Oct 13, 2025

Fix not thread safe xmlSchemaParse calls in ZTS builds by calling xml…

c625d91

…SchemaInitTypes during MINIT. See: https://gitlab.gnome.org/GNOME/libxml2/-/issues/930

SpencerMalone force-pushed the fix-libxml-segfault branch from 47ebdfc to c625d91 Compare October 14, 2025 17:28

nielsdos approved these changes Oct 14, 2025

View reviewed changes

nielsdos closed this in f14e5fc Oct 14, 2025

nielsdos added a commit to nielsdos/php-src that referenced this pull request Oct 14, 2025

Remove xmlInitParser() call from DOM

a32687b

Follow-up for phpGH-20150.

nielsdos mentioned this pull request Oct 14, 2025

Remove xmlInitParser() call from DOM #20168

Merged

nielsdos added a commit that referenced this pull request Oct 15, 2025

Remove xmlInitParser() call from DOM (#20168)

c42fcec

Follow-up for GH-20150.

SpencerMalone mentioned this pull request Oct 20, 2025

Main process exited, code=killed, status=11/SEGV php/frankenphp#1760

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix not thread safe xmlSchemaParse calls in ZTS builds by calling xmlSchemaInitTypes during MINIT #20150

Fix not thread safe xmlSchemaParse calls in ZTS builds by calling xmlSchemaInitTypes during MINIT #20150

SpencerMalone commented Oct 12, 2025

Uh oh!

nielsdos left a comment

Uh oh!

SpencerMalone commented Oct 12, 2025

Uh oh!

SpencerMalone commented Oct 12, 2025

Uh oh!

nielsdos commented Oct 13, 2025

Uh oh!

SpencerMalone commented Oct 13, 2025 •

edited

Loading

Uh oh!

nielsdos commented Oct 13, 2025

Uh oh!

SpencerMalone commented Oct 14, 2025

Uh oh!

nielsdos left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fix not thread safe xmlSchemaParse calls in ZTS builds by calling xmlSchemaInitTypes during MINIT #20150

Fix not thread safe xmlSchemaParse calls in ZTS builds by calling xmlSchemaInitTypes during MINIT #20150

Conversation

SpencerMalone commented Oct 12, 2025

Uh oh!

nielsdos left a comment

Choose a reason for hiding this comment

Uh oh!

SpencerMalone commented Oct 12, 2025

Uh oh!

SpencerMalone commented Oct 12, 2025

Uh oh!

nielsdos commented Oct 13, 2025

Uh oh!

SpencerMalone commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nielsdos commented Oct 13, 2025

Uh oh!

SpencerMalone commented Oct 14, 2025

Uh oh!

nielsdos left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SpencerMalone commented Oct 13, 2025 •

edited

Loading